Add HTTP transport and Bearer token auth for Google Cloud Run deployment - #11
Add HTTP transport and Bearer token auth for Google Cloud Run deployment#11hemati wants to merge 5 commits into
Conversation
…ort and Bearer Token authentication
📝 WalkthroughWalkthroughAdds authenticated Reddit messaging, configurable FastMCP HTTP or stdio execution, optional Bearer-token middleware, deployment dependencies, Cloud Run documentation, and runtime safeguards for Reddit responses. ChangesReddit MCP runtime and deployment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant Uvicorn
participant BearerTokenMiddleware
participant FastMCP
participant RedditAPI
MCPClient->>Uvicorn: Send HTTP MCP request
Uvicorn->>BearerTokenMiddleware: Forward request
BearerTokenMiddleware->>BearerTokenMiddleware: Validate Bearer token
BearerTokenMiddleware->>FastMCP: Dispatch authorized request
FastMCP->>RedditAPI: Verify recipient or perform Reddit operation
RedditAPI-->>FastMCP: Return result or mapped error
FastMCP-->>MCPClient: Return MCP response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server.py (1)
155-208: Consider constant-time comparison for Bearer token validation.The Bearer token authentication middleware is well-structured with proper error responses (401 for missing/invalid format, 403 for wrong token). However, the token comparison at line 192 uses standard string comparison, which may be vulnerable to timing attacks.
🔎 Proposed fix using secrets.compare_digest
Add import at the top of the file:
import secretsThen update the token comparison:
# Extract and validate token token = auth_header[7:] # Remove "Bearer " prefix - if token != self.bearer_token: + if not secrets.compare_digest(token, self.bearer_token): return JSONResponse( status_code=403, content={"error": "Invalid bearer token"} )
secrets.compare_digestperforms constant-time comparison, making timing attacks significantly harder.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ProcfileREADME.mdpyproject.tomlrequirements.txtserver.py
🧰 Additional context used
🪛 Gitleaks (8.30.0)
README.md
[high] 291-296: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
README.md
[grammar] ~262-~262: Ensure spelling is correct
Context: ...} } } } ``` ### Local Testing **Stdio mode (default, for local MCP clients):*...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.14.10)
server.py
140-140: Possible binding to all interfaces
(S104)
1432-1432: Abstract raise to an inner function
(TRY301)
1432-1432: Avoid specifying long messages outside the exception class
(TRY003)
1537-1537: Abstract raise to an inner function
(TRY301)
1537-1537: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (9)
README.md (1)
173-342: Well-documented Cloud Run deployment guide.The deployment instructions are comprehensive and cover:
- Prerequisites and authentication setup
- Basic and secure (Bearer token) deployment options
- MCP client configuration examples
- Local testing for both stdio and HTTP modes
- Monitoring and operational commands
The documentation structure is clear and provides actionable commands.
Procfile (1)
1-1: LGTM! Simple and correct.The Procfile correctly defines the web process for Heroku-style deployments. The server.py main block will handle transport selection based on the
MCP_TRANSPORTenvironment variable.server.py (7)
9-9: Good addition for type checking.Explicitly importing
praw.modelsimproves type annotations and IDE support, especially for the updated_format_commentsignature.
138-152: Environment-driven configuration is well-designed.The FastMCP configuration correctly reads from environment variables:
HOSTdefaults to0.0.0.0(appropriate for Cloud Run containers)PORTdefaults to8080(Cloud Run standard)MCP_STATELESSenables stateless HTTP mode for serverless deploymentsThe static analysis warning about binding to all interfaces (0.0.0.0) is a false positive here—Cloud Run containers require this to accept traffic.
332-332: Improved type annotation for flexibility.Updating the type hint to accept both
praw.models.Commentandpraw.models.Messageimproves type safety and correctly reflects that the function can handle both types of objects.
1431-1432: Good defensive check for API response.Adding validation that
replyis not None prevents potential issues if the Reddit API fails silently. The explicit error message helps with debugging.
1536-1537: Good defensive check for API response.Consistent with the pattern in
reply_to_post, this defensive check ensures the Reddit API returned a valid response.
1906-1908: Good fix for potential UnboundLocalError.Initializing
clean_submission_idbefore the try block ensures it's defined if_extract_reddit_idraises an exception, preventing an UnboundLocalError when the exception handler references it at line 2038.
2402-2434: Well-structured entry point with proper transport selection.The
__main__block correctly handles three modes:
- HTTP with Bearer token authentication (manual uvicorn with middleware)
- HTTP without authentication (standard mcp.run)
- Stdio mode (backward-compatible default)
The conditional logic is clear, and logging messages help with debugging deployment issues.
…erver.py with detailed docstrings for better clarity
hemati
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server.py (1)
184-231: Consider using constant-time comparison for bearer token validation.The bearer token comparison at line 225 uses a standard equality check, which may be vulnerable to timing attacks. For security-sensitive token validation, consider using a constant-time comparison function.
🔎 Proposed fix using secrets.compare_digest
Add import at the top of the file:
+import secrets import functools import loggingThen update the token comparison:
# Extract and validate token token = auth_header[7:] # Remove "Bearer " prefix - if token != self.bearer_token: + if not secrets.compare_digest(token, self.bearer_token): return JSONResponse( status_code=403, content={"error": "Invalid bearer token"} )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyproject.tomlrequirements.txtserver.py
🚧 Files skipped from review as they are similar to previous changes (1)
- requirements.txt
🧰 Additional context used
🪛 Ruff (0.14.10)
server.py
158-158: Possible binding to all interfaces
(S104)
1465-1465: Abstract raise to an inner function
(TRY301)
1465-1465: Avoid specifying long messages outside the exception class
(TRY003)
1570-1570: Abstract raise to an inner function
(TRY301)
1570-1570: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (7)
server.py (7)
9-9: LGTM!Explicit import of
praw.modelsimproves type checking and makes type annotations clearer.
28-32: LGTM!The docstring additions improve code documentation and follow Python best practices.
Also applies to: 128-140
156-170: LGTM! Configuration appropriate for Cloud Run deployment.The environment-based configuration is well-structured. Binding to
0.0.0.0(flagged by static analysis) is intentional and necessary for Cloud Run deployments to accept external traffic. The stateless HTTP mode is appropriate for serverless environments.
365-365: LGTM!The type annotation improvement makes the function's accepted types explicit and enhances type safety.
1464-1465: LGTM! Good defensive programming.The checks for
Nonereplies add robustness by ensuring Reddit API calls succeeded before proceeding. This prevents silent failures and provides clear error messages.Also applies to: 1569-1570
1939-1941: LGTM! Good fix for potential UnboundLocalError.Initializing
clean_submission_idbefore the try block ensures the variable is always bound when referenced in the exception handler, preventing potential errors.
2436-2467: LGTM! Well-structured transport selection and server startup.The implementation properly handles both HTTP and stdio transports, with backward compatibility maintained through the stdio default. The conditional middleware injection for Bearer token authentication is correctly implemented, and logging provides good visibility into the server's configuration.
|
@Arindam200 this is a larger PR, but it adds an important new feature: using the MCP over HTTP. |
who_am_i was broken on praw >= 8: current_user.subreddit now returns a UserSubreddit object rather than a dict, so the nine .get() calls below it failed with "'UserSubreddit' object has no attribute 'get'". The object is now converted to a dict first; the surrounding code is unchanged. send_message(username, subject, message) is new, modelled on reply_to_comment and gated by @require_write_access. It resolves the recipient before sending so a typo fails locally instead of silently at Reddit, enforces Reddit's 100 character subject limit and 1..10000 for the body, accepts u/name, /u/name and name alike, and translates rate limits, blocked recipients and unknown users into readable errors. Reddit returns no message id on success, which the result notes. Delivery itself is untested: Reddit refuses messages to the authenticated account itself (NOT_WHITELISTED_BY_USER_MESSAGE), and any other recipient would have been a real person. Everything up to and including the API call is verified, as is every validation path. Also ignore venv/ and env/. Only .venv was listed, so a `git add -A` in a checkout using venv/ would have committed the whole virtual environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011vLKdeiPdvNQYxJkXw5dYx
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 107-111: Update the send_message documentation to state that
self-messages trigger a warning and are still sent, matching server.py behavior;
alternatively, change send_message to reject self-messages and preserve the
existing refusal statement.
In `@requirements.txt`:
- Around line 1-4: Add starlette>=0.49.1 to the dependency list and update
server.py startup so configuring MCP_BEARER_TOKEN without Starlette raises an
error instead of clearing bearer_token or starting unauthenticated; preserve
normal startup when authentication is not configured.
In `@server.py`:
- Around line 1647-1653: Update the recipient lookup exception handling around
redditor.id to catch only prawcore.exceptions.NotFound and raise the existing
nonexistent-user ValueError from it; allow all other lookup exceptions to
propagate to the existing outer handler so they become RuntimeError.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b09fa70a-a78c-4561-a4fc-c6f388c0bfd2
📒 Files selected for processing (5)
.gitignoreREADME.mdpyproject.tomlrequirements.txtserver.py
| - `send_message(username, subject, message)` - Send a private message to a user. | ||
| Subject is capped at 100 characters by Reddit, body at 10000. The recipient is | ||
| verified before sending, so a typo fails locally instead of silently at Reddit. | ||
| Note that Reddit returns no message id on success and refuses messages to the | ||
| authenticated account itself. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the self-message behavior statement.
The README says that the tool refuses messages to the authenticated account. server.py only logs a warning and then sends the message. Change this text to state that the tool warns, or reject self-messages in send_message.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 107 - 111, Update the send_message documentation to
state that self-messages trigger a warning and are still sent, matching
server.py behavior; alternatively, change send_message to reject self-messages
and preserve the existing refusal statement.
| mcp[cli]>=1.23.0 | ||
| praw>=7.8.1 | ||
| uvicorn>=0.40.0 | ||
| fastmcp>=2.14.1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the dependency installation path used by deployment.
rg -n -C 3 \
'requirements\.txt|pip(3)? install|poetry install|uv sync|\.\[auth\]|fastmcp|starlette|MCP_BEARER_TOKEN' \
-g 'Procfile' \
-g 'Dockerfile*' \
-g '*.yml' \
-g '*.yaml' \
-g 'README.md' \
-g 'pyproject.toml' \
-g 'requirements.txt' \
-g 'server.py' . || trueRepository: Arindam200/reddit-mcp
Length of output: 4088
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- dependency manifests ---'
cat -n requirements.txt
sed -n '1,35p' pyproject.toml
printf '%s\n' '--- deployment and auth instructions ---'
sed -n '215,245p' README.md
sed -n '275,305p' README.md
printf '%s\n' '--- server authentication branch ---'
sed -n '1,25p' server.py
sed -n '165,245p' server.py
printf '%s\n' '--- relevant references ---'
rg -n -C 3 'requirements\.txt|pyproject\.toml|MCP_BEARER_TOKEN|starlette|fastmcp' \
Dockerfile Procfile README.md pyproject.toml requirements.txt server.py 2>/dev/null || trueRepository: Arindam200/reddit-mcp
Length of output: 9422
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- middleware usage and startup path ---'
rg -n -C 6 'BearerTokenMiddleware|middleware|run\(|MCP_TRANSPORT|streamable-http|http_app|app' server.py
printf '%s\n' '--- server structure ---'
ast-grep outline server.py --view compactRepository: Arindam200/reddit-mcp
Length of output: 13107
Make bearer-token authentication fail closed.
When MCP_BEARER_TOKEN is set but Starlette is unavailable, server.py clears bearer_token. HTTP startup then runs without authentication middleware. Make startup fail and add starlette>=0.49.1 to requirements.txt or deploy with .[auth].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@requirements.txt` around lines 1 - 4, Add starlette>=0.49.1 to the dependency
list and update server.py startup so configuring MCP_BEARER_TOKEN without
Starlette raises an error instead of clearing bearer_token or starting
unauthenticated; preserve normal startup when authentication is not configured.
| try: | ||
| recipient_id = redditor.id | ||
| except Exception as lookup_error: | ||
| logger.exception(f"Recipient u/{clean_username} not found: {lookup_error}") | ||
| raise ValueError( | ||
| f"User u/{clean_username} does not exist or is not reachable" | ||
| ) from lookup_error |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For PRAW version 7.8.1, which exception classes can reddit.redditor(name).id raise for a nonexistent account, HTTP request failure, and forbidden access? Use official PRAW or prawcore documentation and source.
💡 Result:
In PRAW version 7.8.1, accessing the id attribute of a lazy Redditor object triggers a network request, which can raise specific exceptions from the prawcore library when the request fails or encounters an error [1][2]. These exceptions are part of prawcore, the low-level communication layer used by PRAW [3][4]. The following prawcore.exceptions classes are raised in these scenarios: 1. Nonexistent Account: When attempting to access the id of a nonexistent user, PRAW raises prawcore.exceptions.NotFound [5][2]. This indicates a 404 HTTP response from Reddit's API [6][4]. 2. Forbidden Access: When access is forbidden (e.g., attempting to access data that the current user does not have permission to view, or interacting with a restricted/banned account), PRAW raises prawcore.exceptions.Forbidden [7][8]. This corresponds to a 403 HTTP response [7][6]. 3. HTTP Request Failure: General network or HTTP-level failures are typically raised as specific subclasses of prawcore.PrawcoreException [9][10]. Common exceptions include: * prawcore.exceptions.RequestException: Raised for errors occurring during the attempt to make the HTTP request (e.g., connection errors) [9][6]. * prawcore.exceptions.ServerError: Raised when a 5xx HTTP response is received from the server [9][6]. * prawcore.exceptions.TooManyRequests: Raised when hitting rate limits (429 HTTP status) [6][4]. To handle these, you should import the exceptions directly from prawcore.exceptions [5][7]. PRAW itself does not wrap all prawcore exceptions, so catching them directly is the standard practice [7]. Example usage: from prawcore.exceptions import NotFound, Forbidden, RequestException try: user_id = reddit.redditor("nonexistent_user").id except NotFound: print("User does not exist.") except Forbidden: print("Access is forbidden.") except RequestException: print("A network or HTTP request error occurred.")
Citations:
- 1: https://www.reddit.com/r/redditdev/comments/5wrtnf/praw_whats_the_recommended_way_of_checking_a/
- 2: https://stackoverflow.com/questions/65116356/prawcore-exceptions-notfound-received-404-http-response
- 3: https://github.com/praw-dev/prawcore
- 4: https://github.com/praw-dev/prawcore/blob/main/prawcore/sessions.py
- 5: https://stackoverflow.com/questions/59697400/check-if-user-exists-with-praw
- 6: https://github.com/praw-dev/prawcore/blob/master/prawcore/sessions.py
- 7: Exception in requests not catchable praw-dev/praw#1625
- 8: https://www.exchangetuts.com/how-to-fix-the-forbidden-received-403-http-response-in-praw-1766665202421388
- 9: https://github.com/praw-dev/prawcore/blob/main/prawcore/exceptions.py
- 10: https://github.com/praw-dev/prawcore/blob/master/prawcore/exceptions.py
🏁 Script executed:
sed -n '1,120p' server.py
sed -n '1580,1720p' server.py
rg -n "prawcore|NotFound|Forbidden|RequestException|ServerError|TooManyRequests|RuntimeError|does not exist or is not reachable" server.py pyproject.toml requirements*.txt uv.lock 2>/dev/nullRepository: Arindam200/reddit-mcp
Length of output: 16215
🏁 Script executed:
printf '%s\n' '--- imports ---'
sed -n '1,120p' server.py
printf '%s\n' '--- send_message ---'
sed -n '1580,1720p' server.py
printf '%s\n' '--- related symbols ---'
rg -n "prawcore|NotFound|Forbidden|RequestException|ServerError|TooManyRequests|RuntimeError|does not exist or is not reachable" server.py pyproject.toml requirements*.txt uv.lock 2>/dev/null || trueRepository: Arindam200/reddit-mcp
Length of output: 16276
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import re
text = Path("uv.lock").read_text()
for name in ("praw", "prawcore"):
match = re.search(rf'(?ms)^name = "{name}"\nversion = "([^"]+)"', text)
print(f"{name}={match.group(1) if match else 'not found'}")
PY
python3 - <<'PY'
from urllib.request import urlopen
urls = [
"https://raw.githubusercontent.com/praw-dev/praw/v7.8.1/praw/models/redditor.py",
"https://raw.githubusercontent.com/praw-dev/prawcore/v2.4.0/prawcore/exceptions.py",
]
for url in urls:
print(f"--- {url} ---")
try:
body = urlopen(url, timeout=10).read().decode()
for i, line in enumerate(body.splitlines(), 1):
if "class Redditor" in line or "def id" in line or "class NotFound" in line or "class Forbidden" in line or "class RequestException" in line:
print(f"{i}: {line}")
except Exception as exc:
print(type(exc).__name__, exc)
PYRepository: Arindam200/reddit-mcp
Length of output: 650
Handle recipient lookup exceptions by type.
Catch prawcore.exceptions.NotFound and raise the nonexistent-user ValueError. Let other lookup exceptions reach the existing outer handler, which converts them to RuntimeError.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server.py` around lines 1647 - 1653, Update the recipient lookup exception
handling around redditor.id to catch only prawcore.exceptions.NotFound and raise
the existing nonexistent-user ValueError from it; allow all other lookup
exceptions to propagate to the existing outer handler so they become
RuntimeError.
2 participants
This pull request adds support for deploying the Reddit MCP server to Google Cloud Run using HTTP transport, including optional Bearer Token authentication for secure access. The changes also improve local development and testing workflows, update dependencies to support HTTP serving, and enhance error handling and logging throughout the codebase.
Deployment and Authentication Enhancements:
README.mdfor deploying to Google Cloud Run, including environment variable configuration, authentication setup, and log monitoring.MCP_BEARER_TOKENenvironment variable and implemented using Starlette middleware.__main__section inserver.pyto support both stdio and HTTP transports, with conditional middleware injection for authentication and Uvicorn server startup for HTTP mode.Procfileto specify the web server startup command for deployment platforms.Dependency and Configuration Updates:
pyproject.tomlto addfastmcp,uvicorn, and an optionalauthdependency group for Starlette-based authentication.praw.modelsis explicitly imported for type checking and improved type annotations for comment formatting. [1] [2]Robustness and Logging Improvements:
Summary by CodeRabbit
New Features
Documentation
Chores